Refactor the templating engine and the command tool cache et al. #615
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use an allocation-free implementation of the ParamKey type. Fix formatting in BuildCommand.fs and syntax highlighting in _template.html.
It uses spans. For the most of allocation savings, FSharp.Formatting.Common now targets .NET Standard 2.1 too but the package is not yet updated; only the command tool will take advantage of all improvements.
Remove an unused source file. And use LINQ-to-XML to generate version.props.
The cache is both more secure and more compact (the cache's size for this project was reduced by a third, after using binary XML). And use a more reliable way to install the command tools during documentation generation.
teo-tsirpanis
commented
Oct 14, 2020
dsyme
reviewed
Oct 15, 2020
| @@ -1,7 +1,7 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <Project Sdk="Microsoft.NET.Sdk" ToolsVersion="15.0"> | |||
| <PropertyGroup> | |||
| <TargetFramework>netstandard2.0</TargetFramework> | |||
| <TargetFrameworks>netstandard2.0; netstandard2.1</TargetFrameworks> | |||
dsyme
reviewed
Oct 15, 2020
| /// An abbreviation for 'string' representing a strong name for a parameter key | ||
| type ParamKey = string | ||
| #endif | ||
| with |
There was a problem hiding this comment.
(And will not compile in release mode??)
Yes just multi-target them, or move them all to .NET Standard 2.1 (probably ok?) |
|
Thanks, this looks great. Just move everything to netstandard 2.1 I think? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR:
DataContractSerializerinstead of the insecureBinaryFormatter. The XML output of the serializer is encoded in binary to reduce its size.Templating.fsto use a more efficientSpan-based single-pass algorithm.Options.fs, making them allocate less.AssemblyInfo.?sandversion.props, the latter of which being now generated with LINQ-to-XML.As a consequence of the templating engine rewrite, the
FSharp.Formatting.Commonpackage multi-targets to .NET Standard 2.1 in addition to 2.0, to take advantage of theStringBuilder.Appendoverload that takes aReadOnlySpan(in .NET Standard 2.0 the span's content will be first copied to a string). The command tool will automatically use the .NET Standard 2.1 version.However, the other public packages -namely
FSharp.FormattingandFSharp.Formatting.Literate- still target only .NET Standard 2.0 because of concerns about the packages' size being doubled. Should I multi-target them as well?